Repository navigation
Name the MultivariateNormal covariance cov in params - #438
Merged
Merged
Conversation
MultivariateNormal.params returned the covariance matrices under the key
scale. Everywhere else in ngboost scale is a standard deviation, so code
that reads params['scale'] can treat a covariance as one. One public
project passes the diagonal of params['scale'] to scipy.stats.norm as a
standard deviation. params now uses the key cov, the name that
scipy.stats.multivariate_normal and the cov attribute of the distribution
use. loc does not change.
params is now a small dict subclass. params['scale'] and
params.get('scale') still return the covariance, but they give a
FutureWarning that says it is a covariance and names cov. scale is not
stored as a key, so keys(), items() and the repr show only loc and cov,
and the keys will not change again when the alias goes.
Found while assessing #168.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ryan-wolbeck
self-requested a review
November 7, 2026 23:26
ryan-wolbeck
approved these changes
Nov 7, 2026
Resolve the RELEASE_NOTES.md conflict with the 0.6.0 rewrite in #461: the cov note now sits in the 0.6.0 "New features" section. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
alejandroschuler
added a commit
that referenced
this pull request
Nov 10, 2026
#438 documented params for a batch of N rows. dist[i] gives loc of shape p and cov of shape p x p. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I found this while assessing #168 (see #433).
Problem
MultivariateNormal(k).paramsreturns the covariance matrices under the keyscale. Everywhere else in ngboost,scaleis a standard deviation:Normal,Laplace,TandCauchyall use it that way. So it is easy to readparams["scale"]as a standard deviation. This happens in public code. jrharbin-york/SBTPredictors computes intervals withstats.norm.interval(interval_val, loc=locs[i][j], scale=scales[i][j][j]), wherescales = y_dist_test.params["scale"]. That passes a variance where scipy expects a standard deviation, so the intervals are too wide where the variance is above 1 and too narrow where it is below 1.The name also does not match
scipy.stats.multivariate_normal, which calls the covariancecov. The distribution class itself already has acovattribute.Design
paramsnow returns{"loc": ..., "cov": ...}. The values and their order do not change, only the second key.To keep old code running for at least one release,
paramsis a smalldictsubclass,MVNParams, defined inmultivariate_normal.py.params["scale"]andparams.get("scale")still return the covariance, but they give thisFutureWarning, which points at the caller's line:scaleis not stored as a key. It is handled in__missing__andget. Sokeys(),items(), iteration and the repr show onlylocandcov. The keys change once, now, and do not change again when the alias is removed.locstays. It is the key that every location family in ngboost uses, it matches thelocattribute of the class, and it is correct: it is the mean vector. Third-party code readsparams["loc"](see below).Alternatives I rejected
covand keepscaleas a silent alias, with a note in the docstring. Nobody reads the docstring of a property they already use, so the code above would never learn that it reads a covariance. The repr would also print every covariance matrix twice.scaleas a stored key that warns when read. The repr would still print the matrices twice and advertise the wrong name. Generic code that loops over the keys, such as the sample-weight test intests/test_basic.py, would get a warning for a key it did not choose. The keys would also change a second time at removal.meanandcovkeys so thatscipy.stats.multivariate_normal(**params)works. This cannot work with any key names.multivariate_normaltakes one mean vector and one covariance matrix and does not broadcast over rows. With scipy 1.18.1, both the N-row arrays and a one-row slice (dist[0:1].params) raiseValueError. Ameankey would only duplicateloc. Row by row,multivariate_normal(mean=params["loc"][i], cov=params["cov"][i])works, andscipy_distribution()already returns these N objects. The new docstring says so.loctomean. This would break every reader ofparams["loc"]for no gain in clarity.Who reads these keys
In this repo, nothing reads
params["scale"]of aMultivariateNormal:ngboost/does not readparamsat all.examples/multivariate_normal.pyreadsy_dist.cov.notebooks/example_multivariate_normal.ipynbusesdists.locandscipy_distribution().docs/only showsparamsfor univariate distributions. None of these need a change.tests/test_basic.pyloops overparams.items(), but only forNormal.On GitHub (
gh search code, which covers default branches only):params["loc"]andparams["scale"]and passes them tomultivariate_normalas the mean and covariance. This is correct. It keeps working and gets the warning.dist.covor callscipy_distribution(). They are not affected.skpro/regression/adapters/ngboost/_ngboost_proba.py) map Normal, Laplace, TDistribution, Poisson, LogNormal and Exponential. They do not supportMultivariateNormal.What can break
"scale" in paramsis nowFalse, andparams.keys()givesloc, cov. Code that uses the key namescalefrom iteration, or callsdict(params)["scale"]orparams.pop("scale"), gets aKeyError. I found no such code.k, andparamsis built at prediction time. Theparamsdict itself also pickles, and the copy keeps the alias.Example
Tests
test_multivariatenormal_params_cov(tests/test_distns.py) builds a distribution from a known covariance and checks thatparams["cov"]equals it, equalsdist.cov, and gives the same log density throughscipy.stats.multivariate_normalasdist.logpdf.test_multivariatenormal_params_scale_is_deprecatedchecks thatparams["scale"]andparams.get("scale")return the covariance with aFutureWarningthat points at the caller, and thatloc,cov,items()and unknown keys behave as in a plain dict, with no warning.test_mvn_params_after_pickle(tests/test_pickling.py) checks that a pickledMultivariateNormalmodel gives the samelocandcov, and that the pickledparamsdict keeps the alias.--slow, the 14MultivariateNormaltests intests/test_distns.pyandtests/test_score.pypass.black24.10.0,isort5.13.2,flake8andpylint(options from.pre-commit-config.yaml) are clean on the changed files.Overlap with #433
#433 changes
__getattr__in many distribution files but does not touchmultivariate_normal.py,tests/test_distns.pyortests/test_pickling.py. Both PRs add a line to the 0.5.11 list inRELEASE_NOTES.md. #433 adds its lines at the top of the list and this PR adds its line at the bottom.git merge-treeof this branch and the head of #433 finds no conflict.🤖 Generated with Claude Code